Skip to content

ADE-95: Pulling main from Git actions pane can kill active sessions#511

Merged
arul28 merged 3 commits into
mainfrom
ade-95-pulling-main-from-git-actions-pane-can-kill-active-sessions
Jun 2, 2026
Merged

ADE-95: Pulling main from Git actions pane can kill active sessions#511
arul28 merged 3 commits into
mainfrom
ade-95-pulling-main-from-git-actions-pane-can-kill-active-sessions

Conversation

@arul28
Copy link
Copy Markdown
Owner

@arul28 arul28 commented Jun 2, 2026

Fixes ADE-95

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

Linked Linear issues

ADE   Open in ADE  ·  ade-95-pulling-main-from-git-actions-pane-can-kill-active-sessions branch  ·  PR #511

Summary by CodeRabbit

  • New Features

    • Auto-rebase now pauses when lanes have active chat or terminal sessions, preventing operations during user interaction.
    • Auto-rebase blocks descendant lanes when ancestor lanes have active sessions.
    • Added fallback handling for activity status verification failures.
  • Tests

    • Added test coverage for auto-rebase session-activity gating and blocking behavior.

Greptile Summary

This PR gates the auto-rebase cascade on active chat and terminal session counts, preventing a git pull from silently rebasing a lane while a user is interacting with it. When sessions are present, the lane is set to rebasePending with an informational message, and all descendants are similarly blocked.

  • autoRebaseService.ts: Adds getLaneActivity hook (optional), normalizeActivityCount helper, and getAutoRebaseActivityBlock to pause or surface lookup errors; both new blockedMessage reasons (active_session, activity_lookup) propagate the block to descendant lanes.
  • bootstrap.ts / main.ts: Wire the activity providers behind an autoRebaseActivityReady flag, then call refreshActiveRebaseNeeds(\"activity_services_ready\") to re-evaluate any lanes that were gated during startup.
  • Tests: Four new scenarios cover session-present blocking, lookup failures, malformed/negative counts, and multi-level ancestor propagation.

Confidence Score: 5/5

The change is safe to merge; new session-activity gating is additive and the service falls back gracefully when the activity provider is absent or throws.

The activity-gating logic is well-isolated behind an optional callback, error paths are caught and converted to a safe pending state rather than allowing an unguarded rebase, edge-case inputs (NaN, negative counts) are normalised correctly, and four targeted test cases verify each new code path. No data loss or incorrect rebase can result from this change.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/src/main/services/lanes/autoRebaseService.ts Core logic for session-activity gating; normalizeActivityCount correctly handles NaN/negative/float inputs; blockedMessage extended with two new reasons used for descendant propagation; getAutoRebaseActivityBlock is well-guarded with error fallback.
apps/desktop/src/main/services/lanes/autoRebaseService.test.ts Four new test cases cover active-session pause, lookup failure, malformed counts (NaN/-2), and ancestor-to-descendant propagation; assertions match expected message substrings correctly.
apps/desktop/src/main/main.ts getLaneActivity wired with autoRebaseActivityReady guard; refreshActiveRebaseNeeds called after flag is set; optional chaining on laneTeardownDeps providers is safe as ?? 0 handles the undefined case.
apps/ade-cli/src/bootstrap.ts Mirrors main.ts wiring with the same autoRebaseActivityReady flag and post-ready refresh; consistent with desktop implementation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[onHeadChanged / refreshActiveRebaseNeeds] --> B[processRoot: build cascadeOrder]
    B --> C{For each lane in cascade}
    C --> D{blocked flag set?}
    D -- Yes --> E[setStatus rebasePending\nblockedMessage ancestor reason]
    D -- No --> F{rebaseMode === manual?}
    F -- Yes --> G[blocked=true, reason=manual]
    G --> C
    F -- No --> H{getLaneActivity provided?}
    H -- No --> K[proceed to rebase]
    H -- Yes --> I[getAutoRebaseActivityBlock]
    I --> J{activity lookup}
    J -- Throws --> L[blocked=true\nreason=activity_lookup\n'could not verify']
    J -- activeSessions > 0 --> M[blocked=true\nreason=active_session\n'Auto-rebase paused']
    J -- No sessions --> K
    L --> C
    M --> C
    K --> N[laneService.rebaseStart]
    N --> O{Error?}
    O -- Yes --> P[blocked=true\nconflict or failed]
    O -- No --> Q[laneService.rebasePush → autoRebased]
    P --> C
    Q --> C
Loading

Reviews (4): Last reviewed commit: "Refs ADE-95: Fix activity refresh promis..." | Re-trigger Greptile

@linear-code
Copy link
Copy Markdown

linear-code Bot commented Jun 2, 2026

ADE-95

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview Jun 2, 2026 9:03am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 2, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR introduces session-activity gating for Auto-Rebase, preventing rebase operations on lanes with active chat or PTY sessions. A new optional getLaneActivity dependency is added to the service, with readiness-gated callbacks in both bootstrap and main entry points that return active session counts per lane or throw until services are fully initialized.

Changes

Lane Activity Gating for Auto-Rebase

Layer / File(s) Summary
Lane Activity Contract
apps/desktop/src/main/services/lanes/autoRebaseService.ts
Introduces LaneActivity type with optional activity counters, normalizeActivityCount helper, extended blockedMessage formatting for new block reasons, and updates blockedReason union type to include active_session and activity_lookup.
Service Configuration with Activity Callback
apps/desktop/src/main/services/lanes/autoRebaseService.ts, apps/ade-cli/src/bootstrap.ts, apps/desktop/src/main/main.ts
Extends createAutoRebaseService signature with optional getLaneActivity parameter and implements readiness-gated callbacks in both bootstrap and main that return per-lane active chat/PTY counts via laneTeardownDeps services or throw when autoRebaseActivityReady is false.
Activity-Based Blocking Logic
apps/desktop/src/main/services/lanes/autoRebaseService.ts
Implements getAutoRebaseActivityBlock(laneId) helper that normalizes activity counts, returns blocking reason/message when activity is detected, handles lookup failures, and integrates the check into processRoot to mark lanes as rebasePending with appropriate messages when blocking conditions exist.
Service Readiness Lifecycle
apps/ade-cli/src/bootstrap.ts, apps/desktop/src/main/main.ts
Sets autoRebaseActivityReady = true in both entry points after agent chat service and teardown-dependency wiring completes, unblocking the getLaneActivity callback.
Test Coverage for Activity Gating
apps/desktop/src/main/services/lanes/autoRebaseService.test.ts
Adds per-lane activity test fixtures (laneActivityById, laneActivityFailures), wires getLaneActivity into test service creation, and adds comprehensive test cases verifying auto-rebase pauses for active sessions, handles lookup failures gracefully, ignores malformed activity counts, and cascades pending status to descendant lanes when ancestors are blocked.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Suggested labels

desktop

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title references the specific issue (ADE-95) and accurately describes the main change: preventing auto-rebase from running on lanes with active chat or terminal sessions, which directly addresses the problem of killing active sessions during a git pull.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade-95-pulling-main-from-git-actions-pane-can-kill-active-sessions

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented Jun 2, 2026

@copilot review but do not make fixes

Comment thread apps/desktop/src/main/services/lanes/autoRebaseService.ts
Comment thread apps/ade-cli/src/bootstrap.ts
@arul28 arul28 force-pushed the ade-95-pulling-main-from-git-actions-pane-can-kill-active-sessions branch from c697ae1 to 34b1651 Compare June 2, 2026 08:09
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/desktop/src/main/main.ts`:
- Around line 2341-2351: When autoRebaseActivityReady transitions from false to
true you must requeue any persisted pending head-change (auto-rebase) entries so
they get processed; add logic immediately after the flag flip (also apply the
same after the other flip near the other occurrence) to load the persisted
pending blocks and re-invoke the existing head-change processing path rather
than leaving them stuck. Locate autoRebaseActivityReady and the getLaneActivity
usage and, when setting autoRebaseActivityReady=true, call a new or existing
routine (e.g., requeuePendingAutoRebases or the same head-change handler used
for live events) to iterate persisted pending blocks and enqueue them for
processing; ensure you reference laneTeardownDeps services as needed when
replaying each pending head-change so behavior matches a live event.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3c39d693-9987-4660-b9ca-9bd642621c43

📥 Commits

Reviewing files that changed from the base of the PR and between 950b536 and 34b1651.

📒 Files selected for processing (4)
  • apps/ade-cli/src/bootstrap.ts
  • apps/desktop/src/main/main.ts
  • apps/desktop/src/main/services/lanes/autoRebaseService.test.ts
  • apps/desktop/src/main/services/lanes/autoRebaseService.ts

Comment thread apps/desktop/src/main/main.ts
Comment thread apps/desktop/src/main/main.ts
@arul28 arul28 merged commit 5ca0473 into main Jun 2, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant